fix(loader-overlay)!: errors following a full review of the component (#DS-5482) - #1982

The auto-fix never runs under ng update — fix arrives undefined.

migrations.json declares no "schema" for any entry (0 occurrences in the file), and the CLI invokes each migration with no options at all — @angular/cli/src/commands/update/cli.js:329 calls this.executeSchematic(workflow, migration.collection.name, migration.name) against the signature executeSchematic(workflow, collection, schematic, options = {}). So schema.json's "fix": { "default": true } never reaches the rule.

A consumer runs ng update @koobiq/components@20, every overlay.text / overlay.caption read stays unmigrated, and the log fills with [loader-overlay-signals] would update <file> (run with --fix to apply). Meanwhile README.md:3 ("invoked automatically by ng update") and docs/guides/migration.en.md ("the text and caption reads are rewritten") promise the opposite.

12 of the 18 sibling migrations already guard this, and app-switcher-signals/index.ts:412 spells out why:

// `ng update` invokes migrations with no options at all, and migrations.json declares no schema, so the
// schema default never reaches us — applying the fix is the intended behaviour there.
const fix = options.fix ?? true;

index.spec.ts:37 is async function run(fix: boolean = true) and always passes fix explicitly, so no test covers the ng update call shape.

yarn run check-public-api-any fails on this line — the baseline was not bumped.

The booleanAttribute transform turns transparent into InputSignalWithTransform<boolean, unknown>, a second unknown in the hand-written surface. tools/check-public-api-any/baseline.json still records "loader-overlay": 1.

I ran the tool's own filter (tools/check-public-api-any/index.ts:31-35 — \b(any|unknown)\b on non-generated, non-comment lines) over both revisions:

.github/workflows/api.yml:20 runs check-public-api-any on every pull_request, so the API job exits 1 with The published type surface gained \any` / `unknown`: loader-overlay: 1 → 2`.

yarn run approve-public-api-any

The template rewrite is a raw String.replace over the whole file, so it corrupts unrelated .text / .caption reads.

The parse is used only to discover ref names; the replacement then ignores the AST entirely. And \b matches after a ., so the comment above ("scoped to those exact identifiers") is not true — any path whose last segment equals the ref name matches.

I ran this exact regex in node against a template declaring <kbq-loader-overlay #overlay />. Every one of these was mutated and written to disk under --fix:

SIGNAL_MEMBERS here is ['text', 'caption'] — two of the most common property names in any codebase — which makes this far likelier than for alert-signals' compact / alertStyle. Anchoring with (?<![.\w$]) before the ref would kill the vm.overlay.text case; the loop-variable and text-node cases need the rewrite to walk binding expressions rather than the raw string.

Compound assignments are rewritten into code that does not parse.

Only EqualsToken is treated as a write; every other assignment form falls through to the read branch on line 168 and gets () appended to the left-hand side:

All three are TS2364 / Invalid left-hand side in assignment. The plain = case is deliberately left alone so the consumer gets a clean read-only error; the compound case turns a type error into a file the compiler cannot parse, at a line the schematic itself wrote. index.spec.ts:124 covers only the plain = form.

Widening the guard to ts.isAssignmentExpression(parent) (or just parent.left === node for any binary assignment operator) plus a PrefixUnaryExpression/PostfixUnaryExpression check would return them all to "leave alone".

Separately, the comment three lines down still reads // Read (incl. optional chain `x?.compact`): append `()`. — compact is a KbqAlert member carried over from alert-signals; every other sibling adapted the example to its own member.

This warning fires on the form the migration already fixed, and misses the form it is actually about.

I ran the pattern in node:

The decorator form returns the instance, so the correct read is this.overlay.text() — which is exactly what collectReceivers + classifyAccess already produced (the declaration carries an explicit : KbqLoaderOverlay annotation, so it is a recognised receiver). The user reads "reading one is a double call, e.g. this.overlay().text()", edits the auto-fixed line, and gets TS2349: This expression is not callable. index.spec.ts:73-89 feeds precisely that source and asserts only the rewrite, never that the warning stays silent — so the false warning ships untested.

Meanwhile [^\n;]* cannot cross a newline, so with printWidth: 120 a real viewChild(KbqLoaderOverlay, { read: ElementRef, descendants: true }) wraps and goes unwarned — the one case the message is right about.

Restricting to the lowercase signal-query forms and allowing newlines fixes both: '(?:viewChild|contentChild)\\s*(?:\\.required)?\\s*\\([\\s\\S]{0,200}?\\bKbqLoaderOverlay\\b'.

HTML files get no warning pass at all.

logWarnings and warnReceiverMembers are called only inside the tsPaths loop (lines 431-432) and both operate on the TypeScript AST, so they can never see a template read. This loop calls only migrateTemplate, which rewrites SIGNAL_MEMBERS (text, caption) and nothing else.

So a template with <kbq-loader-overlay #o /> and {{ o.isEmpty }} or [size]="o.spinnerSize" is neither rewritten nor reported:

• with strictTemplates, the consumer's build fails with TS2445 on a member the migration never mentioned;
• without it, {{ o.isEmpty }} now renders the computed() function's source text instead of true/false, because the getter became a signal.

The ref collector already resolves which refs point at an overlay — the same list could drive a PROTECTED_MEMBERS scan over the template text and emit the same per-file warning the .ts path gets. The same gap applies to inline templates.

Receiver scope is text equality over the whole enclosing function, so a shadowing binding is rewritten.

collectReceivers registers a parameter's scope as the entire enclosing function (findAncestor(node, isFunctionLike)), and this compares node.expression.getText(sourceFile) against the receiver text within that span — there is no binding resolution.

render(overlay: KbqLoaderOverlay) {
    this.rows.forEach((overlay) => console.log(overlay.text));
}

The arrow parameter shadows the typed one, but its read sits inside the outer method's span and matches by text → rewritten to overlay.text() → TypeError on a plain row object. Same for a nested function or const of the same name.

Narrower than the template case (#3) since it needs a real name collision, but text / caption on a variable named after the component is a plausible pairing. Trimming the receiver scope when a nested declaration re-binds the same identifier would close it.

parent is typed HTMLElement | null and neither hook guards it.

DefaultDomRenderer2.addClass is el.classList.add(name) (@angular/platform-browser dom_renderer.mjs:617) — it throws on null, it is not a no-op.

• const ref = createComponent(KbqLoaderOverlay, { environmentInjector }); ref.changeDetectorRef.detectChanges(); before the host node is appended — parentElement is null, ngOnInit throws TypeError: Cannot read properties of null (reading 'classList').
• const ref = vcr.createComponent(KbqLoaderOverlay); ref.destroy(); with no CD in between — ngOnInit never ran, parent is still null, and ngOnDestroy throws in removeClass.

Pre-existing, but this hook is rewritten in the diff (elementRef.nativeElement → kbqInjectNativeElement()) and the PR is billed as a full review, so it is signed off now. An early if (!this.parent) return; in both hooks covers it.

This is a byte-identical copy of an export the file already imports from.

packages/schematics/src/utils/typescript.ts:74 exports collectInlineTemplateRanges; diff between the two reports exactly one differing line — the export keyword. And line 7 already reads import { forEachClass, parseTemplate } from '../../utils/typescript';, where forEachClass is imported only to feed this copy (its sole use is line 322).

Seven migrations already import the shared one (app-switcher-signals, button-state-and-styles, button-supported-colors, button-toggle-signals-and-aria, dropdown-demote-overlay, list-tree-multiple-input, navbar-signals-and-aria), as does utils/icon-migration.ts:603.

Fix: change line 7 to import { collectInlineTemplateRanges, parseTemplate } from '../../utils/typescript'; and delete lines 318-353. −36 lines, zero behavior change — and the shared version documents a guarantee this copy silently won't inherit ("A template literal with a ${…} substitution is skipped").

This sentence contradicts three of the four subsections it introduces.

Section 18's subsections are #### Loader overlay (1052), #### Search expandable (1069), #### Split button (1091) and #### Title (1113). Search-expandable, split-button and title are all named in the first-wave list in this same sentence, yet "the second is the one each subsection below belongs to" assigns them to wave two. Loader overlay is named in neither list.

A reader migrating KbqSplitButton is told split-button was reviewed in wave one and, one clause later, that the #### Split button subsection belongs to wave two. And section 18 keeps growing — it went from 3 to 4 subsections in this PR — so every future addition either re-breaks the sentence or forces another rewrite of it. Wave membership is per-subsection data being encoded in a shared intro.

The original one-liner with loader-overlay added is self-consistent and has nothing to go stale:

docs/guides/migration.ru.md:1042 carries the identical defect ("ко второй относится каждый из подразделов ниже") and needs the mirrored change.